Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove duplicate call to $(document).ready #42

Merged
merged 1 commit into from
Oct 9, 2019
Merged

Remove duplicate call to $(document).ready #42

merged 1 commit into from
Oct 9, 2019

Conversation

campbellgoe
Copy link
Contributor

@campbellgoe campbellgoe commented Apr 19, 2019 •
edited by PierreRambaud

Questions Answers
Branch? develop
Description? This seemed to cause a bug for some users who could not reach the checkout page. This duplicate call would register the 'updateCart' event listener twice, therefore making 2 ajax post requests.
Type? bugfix
BC breaks? no
Deprecations? no
Fixed ticket?
How to test? Please check everything during the cart process works fine as we removed an unnecessary piece of code.

@campbellgoe campbellgoe changed the base branch from master to dev April 19, 2019 13:51
@matks
Copy link
Contributor

matks commented Jun 14, 2019

Seems to make sense 👌
@PierreRambaud any idea of a usecase where a double $(document).ready(function () { would be useful ?

@matks
Copy link
Contributor

matks commented Jul 19, 2019

Seems to make sense 👌
@PierreRambaud any idea of a usecase where a double $(document).ready(function () { would be useful ?

@PierreRambaud ping 😄

@campbellgoe
Copy link
Contributor Author

https://learn.jquery.com/using-jquery-core/document-ready/

@PierreRambaud
Copy link
Contributor

PierreRambaud commented Jul 19, 2019 •
edited

I think the problem come from elsewhere.
$(document).ready can be used many times, it's just here to say "Run when dom is fully loaded".
More because sometimes you need global variables available store in the dom.
You can do

$(document).ready(prepareTabs); 
$(document).ready(prepareTables); 
$(document).ready(prepareMenu);

As you said, the problem is this callback is called twice

@campbellgoe
Copy link
Contributor Author

The $(document).ready is nested inside another $(document).ready, it's clearly illogical and should be removed regardless.

@PierreRambaud
Copy link
Contributor

ohhh my bad, didn't see the nesting :D
In this case, yes it's useless :D

PierreRambaud
PierreRambaud previously approved these changes Jul 19, 2019
View reviewed changes
@matks matks changed the title remove duplicate call to $(document).ready Remove duplicate call to $(document).ready Oct 7, 2019
@campbellgoe @matks
remove duplicate call to $(document).ready
9f90e64 
This seemed to cause a bug for some users who could not reach the checkout page.
This duplicate call would register the 'updateCart' event listener twice, therefore making 2 ajax post requests.
@matks
Copy link
Contributor

matks commented Oct 7, 2019

PR rebased ✅

@Robin-Fischer-PS Robin-Fischer-PS self-assigned this Oct 9, 2019
@Robin-Fischer-PS
Copy link

Thanks @gcdeveloper for the PR, it's good for me :)

@matks
Copy link
Contributor

matks commented Oct 9, 2019

Thank you @gcdeveloper and @Robin-Fischer-PS

@matks matks merged commit a90b77c into PrestaShop:dev Oct 9, 2019
@matks matks mentioned this pull request Oct 16, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matks matks matks approved these changes

@PierreRambaud PierreRambaud PierreRambaud left review comments

Assignees

@Robin-Fischer-PS Robin-Fischer-PS

Labels
QA ✔️
Projects
None yet
Milestone
No milestone
4 participants
@campbellgoe @matks @PierreRambaud @Robin-Fischer-PS